Skip to content

replace substitute-based generator with plain closure#69

Merged
donyunardi merged 8 commits intomainfrom
update_check_pick_generator@main
May 7, 2026
Merged

replace substitute-based generator with plain closure#69
donyunardi merged 8 commits intomainfrom
update_check_pick_generator@main

Conversation

@donyunardi
Copy link
Copy Markdown
Contributor

@donyunardi donyunardi commented May 6, 2026 •
edited
Loading

Part of fix for https://github.com/insightsengineering/coredev-tasks/issues/750

I updated the .check_pick_generator to use plain closure rather than using rlang::new_function + substitute(). Based on my research, rlang::new_function() + substitute() hides the generated function body from covr, resulting in 0% coverage for helpers.R, even though we already have unit tests for is_pick_multiple, is_pick_fixed, and is_pick_ordered.

The plain closure return the exact same thing with previous solution.

It only returns 37.5% but I guess this is covr limitation when using factory pattern solution.

image image

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026 •
edited
Loading

badge

Code Coverage Summary

Filename                  Stmts    Miss  Cover    Missing
----------------------  -------  ------  -------  -------------------------------------------------------------
R/as_picks.R                140      21  85.00%   63, 167-171, 182-186, 201-207, 229-231
R/assertion.R                 5       0  100.00%
R/call_utils.R              147      33  77.55%   23-28, 65, 132-138, 259, 278-289, 292, 317-325
R/helpers.R                   9       0  100.00%
R/interaction.R              42      26  38.10%   39-97
R/module_merge.R            266      14  94.74%   303, 310-320, 337, 616
R/module_picks.R            318      23  92.77%   47-53, 71, 109-110, 298-300, 302-306, 429, 478, 516, 528, 536
R/picks.R                   183       1  99.45%   335
R/print.R                    36       2  94.44%   50, 58
R/resolver.R                141      19  86.52%   80-83, 110-118, 284-289
R/tidyselect-helpers.R       29       0  100.00%
R/tm_merge.R                 54      54  0.00%    42-102
R/ui_containers.R            42       0  100.00%
R/zzz.R                       5       5  0.00%    3-11
TOTAL                      1417     198  86.03%

Diff against main

Filename            Stmts    Miss  Cover
----------------  -------  ------  --------
R/call_utils.R          0     +11  -7.48%
R/helpers.R            -3     -12  +100.00%
R/interaction.R         0     +25  -59.52%
R/module_merge.R       +9     +12  -4.48%
R/resolver.R            0      +4  -2.84%
TOTAL                  +6     +40  -2.78%

Results for commit: ee46d0e

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026 •
edited
Loading

Unit Tests Summary

  1 files   10 suites   17s ⏱️
255 tests 241 ✅ 14 💤 0 ❌
383 runs  368 ✅ 15 💤 0 ❌

Results for commit ee46d0e.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026 •
edited
Loading

Unit Test Performance Difference

Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
interaction 💀 $0.22$ $-0.22$ interaction_vars_filters_table_when_specific_values_are_subset

Results for commit 9acc38f

♻️ This comment has been updated with latest results.

@donyunardi
Merge branch 'update_check_pick_generator@main' of github.com:insight…
89d0867 
…sengineering/teal.picks into update_check_pick_generator@main
@donyunardi
Copy link
Copy Markdown
Contributor Author

donyunardi commented May 6, 2026

Roxygen Man Pages Auto Update was triggered, updating/adding the .Rd files.

@donyunardi donyunardi added the core label May 6, 2026
@averissimo averissimo self-assigned this May 6, 2026
@osenan
Copy link
Copy Markdown
Contributor

osenan commented May 6, 2026

I have added a change in merge_srv in order to have coverage for all function files. The module function had a big coverage, but `covr::package_coverage was not picking it.

averissimo
averissimo reviewed May 6, 2026
View reviewed changes
Comment thread R/helpers.R Outdated
@donyunardi donyunardi requested a review from averissimo May 6, 2026 22:12
averissimo
averissimo approved these changes May 7, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@averissimo averissimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@donyunardi donyunardi merged commit d735f14 into main May 7, 2026
27 of 28 checks passed
@donyunardi donyunardi deleted the update_check_pick_generator@main branch May 7, 2026 08:17
@github-actions github-actions Bot locked and limited conversation to collaborators May 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@averissimo averissimo averissimo approved these changes

Assignees

@averissimo averissimo

Labels

core

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@donyunardi @osenan @averissimo